Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move binarySearchFirstItem back to the web/-folder (PR 15237 follow-up) #15313

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

This was moved into the src/display/-folder in PR #15110, for the initial editor-a11y patch. However, with the changes in PR #15237 we're again only using binarySearchFirstItem in the web/-folder and it thus seem reasonable to move it back there.
The primary reason for moving it back is that binarySearchFirstItem is currently exposed in the public API, and we always want to avoid that unless it's either PDF-related functionality or code that simply must be shared between the src/- and web/-folders. In this case, binarySearchFirstItem is a general helper function that doesn't really satisfy either of those alternatives.

…ow-up)

This was moved into the `src/display/`-folder in PR 15110, for the initial editor-a11y patch. However, with the changes in PR 15237 we're again only using `binarySearchFirstItem` in the `web/`-folder and it thus seem reasonable to move it back there.
The primary reason for moving it back is that `binarySearchFirstItem` is currently exposed in the public API, and we always want to avoid that unless it's either PDF-related functionality or code that simply must be shared between the `src/`- and `web/`-folders. In this case, `binarySearchFirstItem` is a general helper function that doesn't really satisfy either of those alternatives.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/7519731c21a8a7d/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/9d92ce556899dc6/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/7519731c21a8a7d/output.txt

Total script time: 26.73 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 9
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/7519731c21a8a7d/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/9d92ce556899dc6/output.txt

Total script time: 28.55 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 2

Image differences available at: http://54.193.163.58:8877/9d92ce556899dc6/reftest-analyzer.html#web=eq.log

@timvandermeij timvandermeij merged commit e6fe127 into mozilla:master Aug 14, 2022
@timvandermeij
Copy link
Contributor

Looks good; thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants